Codecov Report

Merging #5347 (d0d5929) into main (a0cff37) will increase coverage by 0.10%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #5347      +/-   ##
==========================================
+ Coverage   87.56%   87.66%   +0.10%     
==========================================
  Files          76       76              
  Lines       13732    13714      -18     
==========================================
- Hits        12025    12023       -2     
+ Misses       1707     1691      -16     

The doc trick does not seem to work: https://pymc--5347.org.readthedocs.build/en/5347/api/distributions/transforms.html

CC @OriolAbril guru 🧞

There are two issues here. The main one is that we are trying to use pymc.transforms. This is what happens on my computer:

In [1]: import pymc.transforms as transforms
---------------------------------------------------------------------------
ModuleNotFoundError                       Traceback (most recent call last)
<ipython-input-1-dd33c3c1e58a> in <module>
----> 1 import pymc.transforms as transforms

ModuleNotFoundError: No module named 'pymc.transforms'

In [2]: from pymc import transforms

In [3]: transforms.Chain
Out[3]: pymc.distributions.transforms.Chain

In [4]: import pymc.transforms
---------------------------------------------------------------------------
ModuleNotFoundError                       Traceback (most recent call last)
<ipython-input-4-2bd367763092> in <module>
----> 1 import pymc.transforms

ModuleNotFoundError: No module named 'pymc.transforms'

I have no idea how this happens nor how to fix it.

sphinx uses import pymc.transforms to then get the docstrings form the onbjects listed in https://github.com/pymc-devs/pymc/blob/main/docs/source/api/distributions/transforms.rst. As it fails, only the index table at https://pymc--5347.org.readthedocs.build/en/5347/api/distributions/transforms.html is generated, but unlike what happens with continuous distributions for example, you can't click on them to get to the specific page with the whole docstring.

The second much less important issue is that even if that were fixed, docstrings have some constraints because we use numpydoc to parse them, so directives right in the extended description might not work as expected. I'd suggest using sections
defined by numpydoc instead of raw directives, iirc, the warning one is parsed and formatted as an admonition, but the notes are not. Either way, numpydoc also defines the order in which every section appears in the rendered docs independently of the order in the docstring I belive, so if the notes end up at the bottom and we want the tip to appear at the top we can look into it.

forgot some links: https://numpydoc.readthedocs.io/en/latest/format.html#warnings and https://numpydoc.readthedocs.io/en/latest/format.html#notes

@OriolAbril I fixed the module path and the elements are now clickable. It still seems like sphinx ignores the custom __doc__:

https://pymc--5347.org.readthedocs.build/en/5347/api/distributions/generated/pymc.distributions.transforms.Interval.html#pymc.distributions.transforms.Interval

Would it be better to add specific documentation directly in the rst (shivers) file?

It is still being imported in init https://github.com/ricardoV94/pymc3/blob/transform_issue/pymc/__init__.py#L49 to expose pymc.transforms to users, and even though it is now documented as pymc.distributions.transforms, the examples (and cross references) use pymc.transforms instead. I find this very confusing. Imo, If we want users to use pymc.transforms we should document there and try and fix that import issue. The transforms file seems quite standalone, maybe it could be moved outside the distributions folder? (even then we can document it inside distributions in the api, those groupings are conceptual now, most things are imported and documented as pymc.object)

The ignoring of the doc is very confusing. The rest are working, my guess is that somehow doing this trick on classes instead of functions is not possible or has to be done in some other way. And the worse thing is it has a docstring somehow. Where does the alias of aeppl.transforms.intervaltransform come from? It even uses the correct syntax for sphinx cross references?!?!?!!!!

Would it be better to add specific documentation directly in the rst (shivers) file?

We can try that yeah, but then the docstring won't be available from ides which is far from ideal.

had an idea that might fix this? (coming from someone with little understanding of imports)

We now do

in the "global" init, but we also do

and all distributions are available in the pymc namespace. So maybe doing the from pymc.distributions import transforms in the distributions/__init__.py and adding transforms to the __all__ variable will work if we then keep only the from distributions import * in the global init

(assuming we do want to keep the transforms inside the distributions folder)

@OriolAbril sounds like it could work

@OriolAbril does my last commit do what you were suggesting?

Docs not good yet: https://pymc--5347.org.readthedocs.build/en/5347/api/distributions/generated/pymc.transforms.Interval.html

That was my idea but transforms continues to not be a proper module:

In [1]: import pymc.transforms as transforms
---------------------------------------------------------------------------
ModuleNotFoundError                       Traceback (most recent call last)
Input In [1], in <module>
----> 1 import pymc.transforms as transforms

ModuleNotFoundError: No module named 'pymc.transforms'

In [2]: from pymc import transforms

In [3]:

I tried updating the transforms.rst file to change currentmodule:: pymc.transforms by pymc only, then list elements as transforms.simplex... instead of only simplex. This partially tricks sphinx, it generates pages for each object, but is still unable to access their __doc__ attribute and the page ends up empty and not properly linked:

I think we can merge for now and at some point fix the import issue so that both import and from ... import work. If that is fixed, the docs will render correctly without any change to the docs themselves.

@OriolAbril would it help if transforms was a proper module itself? With a directory and __init__.py?

I am quite sure it will. Sphinx works as long as it is able to import the objects. If both imports from #5347 (comment) work then sphinx will work, I think making it a proper module will make the two import ways work, but here is where I am not completely sure.

I'll give it a try

Docs are working now: https://pymc--5347.org.readthedocs.build/en/5347/api/distributions/transforms.html. There are some rendering issues though.

@OriolAbril I think formatting is fixed now?

yep

Thanks a lot for the help @OriolAbril. I realized now I should have added you as a co-author :/